Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Route choice docs and adjusting #532

Merged
merged 47 commits into from
Jun 16, 2024
Merged

Conversation

pedrocamargo
Copy link
Contributor

@pedrocamargo pedrocamargo commented Jun 2, 2024

Adds route choice capabilities to AequilibraE. Items still missing:

  • Include treatment of cases where the network is disconnected for a given OD pair
  • Adding documentation for choice set generation (modeling_with_aequilibrae/route_choice/choice_set_generation.rst)
  • Adding example for the creation of route choice sets
  • Adding documentation for route choice (modeling_with_aequilibrae/route_choice/route_choice_model.rst)
  • Add a map with assignment results to the route choice example
  • Make the cutoff probability intuitive (I would expect a value of 0.01 to exclude all routes that have a probability os less than 1%)
  • Update API for route choice (prepare graph with utility and provide only path overlap parameter)

@pedrocamargo pedrocamargo changed the base branch from develop to route_choice June 2, 2024 05:33
pveigadecamargo added 2 commits June 2, 2024 16:08
@janzill
Copy link
Contributor

janzill commented Jun 3, 2024

@Jake-Moss it looks like we calculate path overlap factors before excluding paths with the provided minimum probability parameter. However, we want to exclude these paths from the choice set, which means we want to calculate path overlap factors AFTER exclusion.

@janzill
Copy link
Contributor

janzill commented Jun 3, 2024

We might also want to think about using the minimum probability as an early stopping criterion for path searches.

@Jake-Moss
Copy link
Contributor

@Jake-Moss it looks like we calculate path overlap factors before excluding paths with the provided minimum probability parameter. However, we want to exclude these paths from the choice set, which means we want to calculate path overlap factors AFTER exclusion.

Ah damn, we are too. Shouldn't be a difficult fix

@Jake-Moss Jake-Moss mentioned this pull request Jun 4, 2024
Copy link
Contributor

@janzill janzill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple of minor comments but looking really good

@pedrocamargo
Copy link
Contributor Author

@janzill , since we are saying that utility is positive and the negative sign is in the computation itself, then I think we should do three things:

  • Make sure the math is correct also for the binary logit
  • Document that assumption (needed to compute paths, actually)

pveigadecamargo and others added 4 commits June 12, 2024 23:14
@janzill
Copy link
Contributor

janzill commented Jun 13, 2024

@janzill , since we are saying that utility is positive and the negative sign is in the computation itself, then I think we should do three things:

  • Make sure the math is correct also for the binary logit
  • Document that assumption (needed to compute paths, actually)

Hm, maybe we should refer to cost instead of utility. The cost of a link is a measure that has to be positive for the shortest path algorithm to work, and we need our total cost to be the sum of link costs. The observed utility in the path size logit calculations is then the negative path cost plus parameter times path overlap term (which itself is calculated with link costs). I double-checked, the maths should be correct using cost.

@pedrocamargo pedrocamargo merged commit c02a22d into route_choice Jun 16, 2024
17 checks passed
@pedrocamargo pedrocamargo deleted the pedro/route_choice_docs branch June 16, 2024 09:51
pedrocamargo added a commit that referenced this pull request Jun 19, 2024
* Removed `reached_first` from A*

The current method of skimming for A* produces out of bound accesses for A*.
This occurs when it reconsiders more nodes than are in the network. The
`reached_view` array is primarily for skimming, as such I've disabled skimming
for A*.

Skimming for early exit Dijkstra's should also be reconsidered.

* Add preliminary route choice

* Return noexcepts

* Working RouteChoice generation

* Removed unused variables and junk

* Notes

* fixup! Removed unused variables and junk

* fixup! fixup! Removed unused variables and junk

* Rename files

* Minor fixes

* Use comrpessed graph

* Avoid unnessacary work when we may fill the route set this iteration

* Fix pointer issues, move custom imports

* Spelling

* Prevent memory leak when destination is unreachable

* Remove print out

* Prevent infinite loop when depth is unlimited

* Prevent oob access and incredible memory consumption (2gb/s) from A*

* Scratch testing on Arkansas

* Remove working_set, PO1 makes this redundant, reduce memory usage

Graphs should also been marked as seen regardless of if the destination is reachable

* Remove prints

* Add parallelised batched method for running a list of od pairs

* Remove dead code and fix A* test now that skimming is disabled

* Move `RouteChoice.run` to be wrapper around `RouteChoice.batched`

No need for both methods

* Fix infinite loop in the case that all possible paths are exhausted

* Add comprehensive testing

* Warning clean up

* Update commentary and fix memory leak

* Typos

* Rename and remove imports

* Disable initialisation and bounds checking

* Add docs and support blocking centroid flow with tests

* Remove debug method and fix small typos

* typo

* Allow switching path finding method

* Add debug method for display in tests

* Linting

* Scratch parquet work

* Better structuring for the table

* Add optional immediately disk saving

* Test fixes

* Update CI workflows

In what I consider a bad move, the PyPi wheel installation of pyarrow requires modifying the installation environment to
create symlinks for the linker.

* Move pyarrow to a hard dependency for now

* Linting

* CI again

* Warn about duplicate OD pairs, fix memory corruption, disable A*

* Fix path order

* Add link penalisation

* Linting

* Add parallelised sparse frequency implementation and test

Code is unprofiled, I'm not sure if this is the best approach but it works well.

It works by stacking all the paths in a route set into a big vector, then sorting it. By sorting it all the links become
grouped and we can just count their occurrences. This has the added benefit of the resulting frequency arrays being
sorted so we can bisect then later. Generally this has really simple memory accesses and is easy to read.

Another possible implementation might be to sort each path individually, then walk and merge them all (not adding
duplicates). This is trickier, requires a lot of book keeping to walk n arrays correctly, upside is lower memory usage
provided we sort inplace, if not then it should be the same.

* Remove debug test

* Add route cost computation and test

* Add psl gamma computation, not sure what to test against

* Add scratch probability implementation

* Fix probability calculations

* The great refactor, batch probability computations to dump to disk

Moves the computation of the path sized logit inside the main multithreaded loop, this lets us batch them as well and
dump them to disk along with the reset of the tables. Enable multithreading by default. Catch a handle of memory leaks.

* Revert "Update CI workflows"

This reverts commit e3ea0c6.

* Move the pyarrow symlink creation to setup.py

* CI

* .

* .

* Squashed commit of the following:

commit 9337fb611463606b0ca89d1270210fa7fdf46714
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 11:22:48 2024 +1000

    .

commit 3f2c01b
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 10:09:51 2024 +1000

    I give up

commit c77d5e6
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 10:03:52 2024 +1000

    .

commit 9dc3650
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 09:58:40 2024 +1000

    .

commit b4b945c
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 09:53:10 2024 +1000

    .

commit e0c32f2
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 09:38:29 2024 +1000

    .

commit eedb859
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 09:30:20 2024 +1000

    .

commit d0def11
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 09:27:33 2024 +1000

    .

commit a6b2a8e
Merge: 0548be6 77edeae
Author: Jake Moss <[email protected]>
Date:   Tue Feb 27 09:19:30 2024 +1000

    Merge branch 'develop' into pedro/ci_test

commit 0548be6
Author: Jake-Moss <[email protected]>
Date:   Tue Feb 27 09:13:07 2024 +1000

    Macos test

commit a10c791
Author: Jake-Moss <[email protected]>
Date:   Wed Feb 21 20:00:01 2024 +1000

    Maybe fix MacOS again

commit cad0579
Author: Jake-Moss <[email protected]>
Date:   Wed Feb 21 19:43:00 2024 +1000

    Maybe MacOS fix

commit 24de8f7
Author: Jake-Moss <[email protected]>
Date:   Wed Feb 21 19:31:08 2024 +1000

    Hopefully fix CI

commit 66ffcc3
Author: pveigadecamargo <[email protected]>
Date:   Wed Feb 21 17:01:57 2024 +1000

    .

commit f934c34
Author: pveigadecamargo <[email protected]>
Date:   Wed Feb 21 16:41:36 2024 +1000

    .

commit 9f2ca20
Author: pveigadecamargo <[email protected]>
Date:   Wed Feb 21 16:30:59 2024 +1000

    .

commit ba0d882
Author: pveigadecamargo <[email protected]>
Date:   Wed Feb 21 16:27:14 2024 +1000

    .

commit 315cbce
Author: Pedro Camargo <[email protected]>
Date:   Wed Feb 21 01:01:23 2024 +1000

    Update pyproject.toml

commit d58e7cc
Author: Pedro Camargo <[email protected]>
Date:   Wed Feb 21 00:58:54 2024 +1000

    Update pyproject.toml

commit dd6723f
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 23:02:05 2024 +1000

    .

commit f7ae37e
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 22:58:19 2024 +1000

    .

commit 0f954e4
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 22:54:21 2024 +1000

    .

commit 3dafd88
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 22:27:05 2024 +1000

    .

commit ebe3a19
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 22:12:39 2024 +1000

    .

commit 9f4413e
Merge: daf48a5 2e3c234
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 22:11:22 2024 +1000

    Merge branch 'develop' of github.com:AequilibraE/aequilibrae into pedro/ci_test

commit daf48a5
Merge: 3df73da ccb9cfa
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 22:11:13 2024 +1000

    .

commit 3df73da
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 22:10:13 2024 +1000

    .

commit 9448e76
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 22:10:05 2024 +1000

    adds emulation

commit c9f2aaa
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 21:20:12 2024 +1000

    adds emulation

commit b2d5d3d
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 21:16:51 2024 +1000

    adds emulation

commit 2458f9a
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 21:02:26 2024 +1000

    adds emulation

commit e9e660b
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 20:54:57 2024 +1000

    adds emulation

commit f112262
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 20:42:13 2024 +1000

    Add ARM architectures for Linux and mac

commit bae7d0d
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 20:28:15 2024 +1000

    tests cibuildwheels

commit 293457a
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 20:09:43 2024 +1000

    tests cibuildwheels

commit 06b6a44
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 19:31:21 2024 +1000

    tests cibuildwheels

commit 0ebce09
Author: pveigadecamargo <[email protected]>
Date:   Tue Feb 20 19:23:39 2024 +1000

    tests cibuildwheels

* Re-enable CI

* Scratch working for link loading

* Rudimentary link loading and path file generation

* Fix tests and segfaults

* Scratch comments

* Separate path file generation and link loading

Add method to map compressed link IDs to network IDs

* Fix link ID ordering in compressed -> network mapping

* We don't need functools for this

* Reverse routes during computation, map link IDs during output

* Fix tests

* Fix windows compilation

* Linting

* Add ruff to pre-commit hooks

* Update black pre-commit hook and drop flake8

* Translate link loads from compressed IDs to graph IDs when link

Add decorators as well

* Rename Cython file to avoid name clash

* Add wrapper object and begin API work

* Cannot rely on the ordering of nodes when building the mapping

* Rename gamma -> path_overlap

* Prevent deadend removal + graph compression introducing simple loops

* Move NetworkGraphIndices dataclass, add node mapping, extend API

untested

* Add link to bfsle paper, add American spelling

* Fix lots of small errors in wrapper class

* Merges set algorithm and set parameters. Better docs

* Add example docs and various bug fixes

* Make deadlock case and error, needs a real fix

* Enforce single thread for tests

* Fix the "deadlock", code wasn't deadlocking but it was running away

* Limit pyarrow IO threads, Cython += is funky

* Pyarrow IO threads must be > 0, give tests from more freedom

* Better type checks and some tests

* Fix segfault and infinite loop due to miss count

* Spelling, remove clamping, make algorithm positional or keyword arg

* Forget import

* Skip 3.9 builds

* Revert "Skip 3.9 builds"

This reverts commit a8c72db.

* Drop 3.8 from unit tests

* Don't run off the end of the vector

* Remove FIXMEs, update docs strings, spelling errors

* Add test with known results

* Move graph index building to Cython for free 1.5x

* Add select link support with sparse matrices

* Add select link tests and fix bug

* Add sparse matrix writing

* Update docs, add small api tests

* Add sparse matrix tests and from disk method

* Add link loading and select link results saving

* WIP: add LP to BFSLE, each depth penalises the next depths base graph

* Add optional link penalisation to BFSLE

* Add binary logit cut offs for assignment

* Update tests

* Update example

* Some nicer comments

* Exclude excluded routes from all calculations, not just probability

* Make sure the objects are numpy arrays, not cython memory views

* Route choice docs and adjusting (#532)

* fixes example (#530)

* documentation

* docs

* Changing API

* Docs

* Docs

* Example for choice set generation

* Example for choice set generation

* Image thumbnail for notebook

* Map for example

* Map for example

* .

* Invert probabilities, cut-off now includes only above it, not below

* Clarifies notebook

* Clarifies notebook

* updates CI

* Fix tests for probability cutoff

* Support disconnect OD pairs

* Fix select link not using filtered graph

* Use more copies to avoid link loading issues (hopefully)

* Simplifies return of link loading

* Makes scheduling of parallel jobs more aggressive (each individual job is very quick, so the overhead is negligible and potential for load balance is huge)

* randomizes inputs for load balancing

* removes reference to theta as a utility function parameter

* removes reference to theta as a utility function parameter

* Add missing negation and remove theta parameter from tests

* ci test

* ci test

* Revert "ci test"

This reverts commit a34a497.

* Revert "ci test"

This reverts commit 4eb8cd1.

* CI

* CI

* CI

* CI

* Documentation icons

* Include comments as docs

* Add some detail to the modelling with aeq route choice docs

* response to comments

* .

* .

* string format

* .

* .

* parameter clarification

* move comment one line up for clarity

---------

Co-authored-by: Renata Imai <[email protected]>
Co-authored-by: pveigadecamargo <[email protected]>
Co-authored-by: Jake-Moss <[email protected]>
Co-authored-by: Renata Imai <[email protected]>
Co-authored-by: Jan Zill <[email protected]>

* fixes icon

* Test removing version specifier on auditwheel

* Revert wheel repair test

* Updates pyarrow version

* Bump pyarrow in wheel building

---------

Co-authored-by: Jake-Moss <[email protected]>
Co-authored-by: pveigadecamargo <[email protected]>
Co-authored-by: Jan <[email protected]>
Co-authored-by: Renata Imai <[email protected]>
Co-authored-by: Renata Imai <[email protected]>
Co-authored-by: Jan Zill <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants